-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
contributing: add testsuite tips section #290
Conversation
Sure, I'd be happy to!! I have to admit, pretty much all of my experience with testing under flux is through an interactive Docker container that is built on top of flux-core's Docker container, but if that sounds like something that could be an interesting tutorial section, I can for sure join and talk about that. |
That's exactly my experience, and what we would showcase to external developers, since they wouldn't have access to say, LC resources. But they can spin up the .devcontainer environments in the respective repos and easily run the tests in docker! So that is absolutely what we will showcase - you are set! I haven't set date beyond "after the holiday" so let's sync up after that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to write this up! I made a few suggestions inline.
contributing.rst
Outdated
* Many sharness tests re-run themselves under a Flux instance using the | ||
``test_under_flux`` command. For these tests you can set | ||
``FLUX_TEST_VALGRIND=t`` and all flux-brokers in the test instance will be | ||
run under valgrind for memory debugging. (This is also useful to tease out | ||
race conditions in your test code), e.g.: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_under_flux
is actually a shell function not a command.
Also, I think test_under_flux
is not a prereq for FLUX_TEST_VALGRIND
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh cool!! Thanks for pointing that out.
Perhaps I should separate that note about running tests under valgrind
as its own bullet point.
contributing.rst
Outdated
``flux mini bulksubmit --watch --progress ./{} ::: t[0-9]*.t python/t*.py lua/*.t`` | ||
|
||
* Running all tests in Flux takes some time. If you are developing a feature | ||
and only a few tests are failing make recheck can be useful, it will only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting: make recheck
contributing.rst
Outdated
Testing is an important part of development throughout all of the Flux | ||
components, and any changes or additions contributed should come with adequate | ||
testing. The below examples show how to run tests in a Flux project within | ||
their respective ``t/`` directories. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth mentioning that unit tests are generally co-located with sources and use libtap
, and system tests are generally located under t/
and use sharness
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also there are the python tests that use pycotap
.
Might be helpful to add references to the third party test software mentioned (libtap, pycotap, sharness)
Testing | ||
------- | ||
|
||
Testing is an important part of development throughout all of the Flux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe footnote https://github.com/flux-framework/flux-core/blob/master/t/README.md or add a reference somewhere?
Problem: The Flux projects have very robust and powerful test environments, but there isn't any documentation on how to leverage this test environment for new developers or people who want to contribute to the project for the first time. Add a "Testing" section to the Contributing document on how to run tests within a Flux project and combine it with other tools such as valgrind.
Thanks for taking a look and reviewing this @garlick! I've force-pushed some fixes based on your suggestions above. Setting MWP here |
Problem
The Flux projects have very robust and powerful test environments, but there isn't any documentation on how to leverage this test environment for new developers or people who want to contribute to the project for the first time.
This PR adds a "Testing" section to the Contributing document on how to run tests within a Flux project and combine it with other tools such as
valgrind
. I pretty much just copied the contents from #89 and formatted it into its own subsection. Feel free to push to this branch if there are other tips not included there but could be useful.Fixes #89